media: Add page alignment and lockless decommit - #11550
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
xiaomings
changed the title
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable page alignment option (allocate_with_page_alignment) to the DecoderBufferAllocator and StarboardMemoryAllocator strategies, and updates H5vccSettings to allow configuring this option via a bitmask. The review feedback highlights a backward compatibility issue where existing configurations would silently disable page alignment, suggesting to treat the new bit as a disable flag instead. Additionally, the reviewer recommends using uintptr_t instead of uint8_t* for alignment calculations in StarboardMemoryAllocator to prevent potential compilation issues, and updating the corresponding documentation comments.
…erBufferAllocator Add support for the allocate_with_page_alignment flag to the configurable decommit strategy. This flag is bound to bit 25 (the second bit of the highest byte) of the setting and controls whether the fallback block allocator forces page alignment and page-rounded size. Additionally, implement a lockless suspend decommit optimization by checking the decommit_on_suspend_enabled_ atomic flag before acquiring the mutex. This prevents UI thread blocking on suspend when decommits are disabled. Also simplify StarboardMemoryAllocator pointer arithmetic inside Decommit() using uint8_t*, clean up redundant constructors, and standardize parameter documentation. Bug: 454441375 TAG=agy CONV=92545d5a-4b8a-4031-bbd7-3775e458e496
6289284 to
3b41007
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces an allocate_with_page_alignment option to control page alignment for memory allocations in DecoderBufferAllocator and StarboardMemoryAllocator, alongside a lockless check for decommitting on suspend. The review feedback highlights a backward compatibility risk in parsing the configuration flags that could silently disable page alignment, potential compilation issues from using uint8_t* pointers with alignment functions, and potential test pollution from unconditionally storing true in the atomic decommit_on_suspend_enabled_ flag.
hlwarriner
left a comment
hlwarriner
left a comment
There was a problem hiding this comment.
The third_party/blink/renderer/modules/cobalt/h5vcc_settings/h_5_vcc_settings.cc changes LGTM, as long as web clients are aware of the encoding change for the setting.
Refer to original PR: #11550 Add support for a page alignment flag in DecoderBufferAllocator to control OS block allocation alignment. This allows for more granular memory management when using the configurable decommit strategy. Implement a lockless optimization during suspend by checking an atomic flag before acquiring the allocator mutex. This prevents unnecessary blocking of the UI thread when decommit on suspend is not enabled. Additionally, simplify pointer arithmetic in StarboardMemoryAllocator using uint8_t pointers and update H5vcc settings to expose the new alignment configuration bit. Bug: 454441375 (cherry picked from commit 8ac74bd)
Refer to original PR: #11550 Add support for a page alignment flag in DecoderBufferAllocator to control OS block allocation alignment. This allows for more granular memory management when using the configurable decommit strategy. Implement a lockless optimization during suspend by checking an atomic flag before acquiring the allocator mutex. This prevents unnecessary blocking of the UI thread when decommit on suspend is not enabled. Additionally, simplify pointer arithmetic in StarboardMemoryAllocator using uint8_t pointers and update H5vcc settings to expose the new alignment configuration bit. Bug: 454441375 (cherry picked from commit 8ac74bd)
Refer to original PR: #11550 Add support for a page alignment flag in DecoderBufferAllocator to control OS block allocation alignment. This allows for more granular memory management when using the configurable decommit strategy. Implement a lockless optimization during suspend by checking an atomic flag before acquiring the allocator mutex. This prevents unnecessary blocking of the UI thread when decommit on suspend is not enabled. Additionally, simplify pointer arithmetic in StarboardMemoryAllocator using uint8_t pointers and update H5vcc settings to expose the new alignment configuration bit. Bug: 454441375 (cherry picked from commit 8ac74bd)
3 participants
Add support for a page alignment flag in DecoderBufferAllocator to
control OS block allocation alignment. This allows for more granular
memory management when using the configurable decommit strategy.
Implement a lockless optimization during suspend by checking an atomic
flag before acquiring the allocator mutex. This prevents unnecessary
blocking of the UI thread when decommit on suspend is not enabled.
Additionally, simplify pointer arithmetic in StarboardMemoryAllocator
using uint8_t pointers and update H5vcc settings to expose the new
alignment configuration bit.
Bug: 454441375